test: add renderer client tests for chat_template_kwargs materialization#2711
Open
mvanhorn wants to merge 1 commit into
Open
test: add renderer client tests for chat_template_kwargs materialization#2711mvanhorn wants to merge 1 commit into
mvanhorn wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds renderer-client test coverage proving that
chat_template_kwargsare materialized into the rendered prompt and that omitting them falls back to each model's chat-template default rather than a hardcoded value.Why this matters
Issue #2572 asks for tests confirming that the renderer path in
src/prime_rl/utils/client.pyrespects model-specific defaults and that enabling renderer-based tokenization did not silently break other model families. The cases called out arereasoning_effortfor gpt-oss andenable_thinkingfor qwen, nemotron, and laguna; there was no prior coverage of this behavior.Testing
tests/unit/utils/test_client.pygains a parametrized class covering:reasoning_effortpassed for gpt-oss appearing verbatim in the rendered prompt,enable_thinking=falsesuppressing reasoning markup for qwen/nemotron/laguna, omittingenable_thinkingresolving to the tokenizer chat-template default, an unsupported kwarg surfacing a clear error, and a non-renderer path left unaffected. The change is test-only;src/prime_rl/utils/client.pyis untouched. Covered by the new tests in this PR; full suite runs in CI.Fixes #2572
Note
Low Risk
Test-only changes with no runtime or security impact.
Overview
Adds unit tests in
tests/unit/utils/test_client.pythat exercise the renderer client path (via a stubRendererClientand prompt-capturing fake renderer) to lock in howsampling_args.extra_body.chat_template_kwargsaffect rendering.reasoning_effortfor gpt-oss is expected to show up in the rendered prompt and not be forwarded on the native generatesampling_params.enable_thinkingis covered for Qwen 3.5, Nemotron-3, and Laguna:falsesuppresses reasoning markup; when omitted, behavior follows per-model defaults (parametrized). Passing an unsupported kwarg (e.g.enable_thinkingon gpt-oss) must raise a clearValidationError.A separate test asserts
setup_clientsdoes not attachrenderer_config/renderer_model_nameto the default OpenAI chat completions client when only renderer-related args are passed—guarding the non-renderer path.No production code changes; this is regression coverage for issue #2572.
Reviewed by Cursor Bugbot for commit d1c2edf. Bugbot is set up for automated code reviews on this repo. Configure here.